-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Categories and collections #228
Conversation
…ll-feed into mortenson-feed-per-category
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ✨ great ✨ work as usual @benbalter 😻
Thanks for pushing this forward @benbalter ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Too bad we can't incorporate collection settings with feed settings, since we're basically duplicating that:
collections:
changes:
feed_path: "/feed/ch-ch-ch-changes.xslt"
lib/jekyll-feed/generator.rb
Outdated
prefix = collection == "posts" ? "/feed" : "feed/#{collection}" | ||
if category | ||
"#{prefix}/#{category}.xml" | ||
elsif collections[collection]["path"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do nil checking here in case collections
is nil or collections[collection]
is nil
The settings for this were the toughest part. We could do, e.g.,: collections:
puppies:
feed: true or collections:
puppies:
feed:
path: foo.xml
categories:
- foo But that felt more complex and it would burry feed settings in collection settings, rather than following the pattern of a top-level key named after the plugin. Could see it either way (and could always do both, although that feels SUPER complicated to explain the three ways you could configure it).
|
I would like to review this. Busy at the moment; please just give me a day or so 🙂 |
- changes | ||
``` | ||
|
||
By default, collection feeds will be outputted to `/feed/<COLLECTION>.xml`. If you'd like to customize the output path, specify a collection's custom path as follows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO ..will be output to
reads better..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or active voice: "The default output-path for collection feeds is /feed/<COLLECTION>.xml
"
lib/jekyll-feed/generator.rb
Outdated
# WIll return `/feed/collection.xml` for other collections | ||
# Will return `/feed/collection/category.xml` for other collection categories | ||
def feed_path(collection: "posts", category: nil) | ||
prefix = collection == "posts" ? "/feed" : "feed/#{collection}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is there a leading slash for /feed.xml
but not for feed/#{collection}.xml
.. also, the comments above actually states the opposite..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. That is an oversight. Will push up a fix.
expect(contents).to match /http:\/\/example\.org\/bass\/2014\/03\/04\/march-the-fourth\.html/ | ||
expect(contents).to match /http:\/\/example\.org\/bass\/2014\/03\/02\/march-the-second\.html/ | ||
expect(contents).to match /http:\/\/example\.org\/bass\/2013\/12\/12\/dec-the-second\.html/ | ||
expect(contents).to match /http:\/\/example\.org\/bass\/updates\/2014\/03\/04\/march-the-fourth\.html/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related to this PR.. but IMO enforcing the percent_r
literal on the test files will avoid the need to escape the slashes here..
/cc @DirtyF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Given the time, I'd also love to add unit tests to the generator and meta tag (rather than just relying on integration tests).
That is great! When can we expect this getting merged? |
I hate to be annoying, but I would LOVE to be able to use this feature. It looks like this PR has several approvals. Who's responsible to handle the merge and release? Is there anything a non-maintainer can do to help out here? |
@jekyllbot: +minor |
Thanks @mattmcmanus for the reminder, feel free to use the current master branch if you wanna test the feature before we release a proper gem |
works like a charm, expect a release soon: https://frank.taillandier.me/feed/agile.xml |
I've tried to use this on a Fedora system and it doesn't appear to be working:
I tried all the different permutations for Here is a snippet of my
and from one of the blogs
and I also tried:
A category-specific feed XML simply doesn't appear:
How should somebody troubleshoot in a situation like this? |
This isn't working for me either. I added
to my _config.yml and The feed for the category is being created, but it doesn't have any posts in it. I've tried variations of category in the post, such as Does anyone know why this isn't working? |
@ellenwyllie : |
Does jekyll-feed rely on a certain version of jekyll-assets to work properly? |
@DirtyF |
See also #246. |
This pull request builds on @mortenson's work in #176 to fix #70 by abstracting some of the existing logic out a bit. Specifically, it supports generating feeds for categories (and collections other than posts).
To generate feeds for categories:
And to generate feeds for collections:
Since posts are a collection, it's also possible to generate category feeds for other collections:
Everything is backwards compatible with the existing configuration flags, such that:
Automatically gets converted to:
So this should be a non-breaking change, and theoretically add as little complexity as necessary to the publishing experience (in most cases, just creating a top-level list in the config to make it opt-in).
By default, categories are in the form of
/feed/<category>.xml
, collections are/feed/<collection>.xml
, and collection categories are/feed/<collection>/<category>/xml
.All this is implemented with a small abstraction in the generator, which passes the collection (defaulting to posts) and optionally the category to the existing template.